Support for OO Optimization #21093

WillAyd · 2018-05-16T21:26:01Z

closes pandas is no longer importable with -OO optimization #21071
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jorisvandenbossche

small comment, looks good to me

jorisvandenbossche · 2018-05-16T22:07:11Z

pandas/tests/test_downstream.py

@@ -53,6 +55,15 @@ def test_xarray(df):
    assert df.to_xarray() is not None


+def test_xarray_oo_optimizable():


you can remove the "xarray" here, as it is generally useful to ensure this, not only for xarray

codecov · 2018-05-16T23:16:25Z

Codecov Report

Merging #21093 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #21093      +/-   ##
==========================================
- Coverage   91.84%   91.84%   -0.01%     
==========================================
  Files         153      153              
  Lines       49538    49505      -33     
==========================================
- Hits        45499    45466      -33     
  Misses       4039     4039

Flag	Coverage Δ
#multiple	`90.23% <100%> (-0.01%)`	⬇️
#single	`41.88% <100%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
pandas/util/_decorators.py	`81.96% <100%> (-0.3%)`	⬇️
pandas/tseries/offsets.py	`97% <100%> (ø)`	⬆️
pandas/io/formats/printing.py	`89.38% <0%> (-3.71%)`	⬇️
pandas/core/tools/datetimes.py	`84.43% <0%> (-0.55%)`	⬇️
pandas/core/indexes/datetimelike.py	`96.79% <0%> (-0.1%)`	⬇️
pandas/core/arrays/categorical.py	`95.67% <0%> (-0.01%)`	⬇️
pandas/core/frame.py	`97.22% <0%> (ø)`	⬆️
pandas/core/indexes/base.py	`96.68% <0%> (+0.07%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c85ab08...75dbaad. Read the comment docs.

jreback · 2018-05-16T23:57:04Z

pandas/tseries/offsets.py

@@ -1090,12 +1090,15 @@ def apply(self, other):


 class CustomBusinessMonthEnd(_CustomBusinessMonth):
-    __doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end')
+    if _CustomBusinessMonth.__doc__:
+        __doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end')
    _prefix = 'CBM'


 class CustomBusinessMonthBegin(_CustomBusinessMonth):


I think better to do this with @subsitution and template this. We dont' use this pattern anywhere and might be confusing.

Makes sense - I'll take a look and repush

jreback

also pls add a whatsnew 0.23.1 ok

mattpap · 2018-05-17T13:12:36Z

pandas/tests/test_downstream.py

@@ -53,6 +55,11 @@ def test_xarray(df):
    assert df.to_xarray() is not None


+def test_oo_optimizable():
+    # GH 21071
+    subprocess.check_call(["python", "-OO", "-c", "import pandas"])


Does pandas import all modules at import pandas? If not, maybe this will be helpful.

@WillAyd do we want to go this way to also check some submodules?

I can take a look - if it's not a lot of effort I suppose it doesn't hurt

I've been messing around with this for a bit but AFAICT it keeps getting choked up when trying to import modules containing optional dependencies (ex: pandas.io.s3). I can't think of a simple way to configure the test so that it can differentiate intentional ImportErrors as a result of missing dependencies.

For now I'm going to revert to just working with the top level import - can open a separate issue if we wanted this to work recursively

Yep, if it turns out difficult, on problem in starting with the basic test (which already tests the large majority of our API)

WillAyd · 2018-05-20T20:30:38Z

pandas/util/_decorators.py

-        Name of function to deprecate
-    alternative : str
-        Name of function to use instead
+    name : func


Unless I am overlooking something I didn't see where this decorator was being used, but it seems like there is a mismatch between what was documented and the types it actually accepts (at least looking at alternative).

To make this work with substitution I took the liberty of assuming these arguments should functions and not strings whenever this decorator gets used

Unless I am overlooking something I didn't see where this decorator was being used,

It's not used a lot, but at least it is used for argmin/argmax and for scatter_matrix (it is not used there as a decorator, so I searched for "deprecate("

WillAyd · 2018-05-22T21:43:27Z

The failures here are the result of __doc__ not being mutable for classes in Python27. I did some research on alternatives and there don't appear to be any that are that simple.

I suppose this explains why the Substitution / Appender system wasn't used in the first place for the CustomBusinessMonth classes and I don't see it used on any other classes within the code base (just methods).

@jreback would you be OK with keeping the CustomBusinessMonthStart and CustomBusinessMonthBegin classes as is?

jreback

lgtm

jreback · 2018-05-23T10:57:14Z

doc/source/whatsnew/v0.23.1.txt

@@ -94,3 +94,8 @@ Categorical
 ^^^^^^^^^^^

 -
+
+Other


can you move the tab completion entry here as well (its just below Bug Fixes)

jorisvandenbossche · 2018-05-28T07:15:20Z

pandas/util/_decorators.py

+
+           %(msg)s
+
+        """


I am not sure why, but it seems this does not give the correct docstring.
Can you check that pd.Series.argmin? still gives the deprecation warning at the top? (I checked out this branch and didn't see it, while on master it is there)

jreback

lgtm.

jreback · 2018-05-29T00:56:34Z

pandas/tseries/offsets.py

@@ -1090,12 +1090,17 @@ def apply(self, other):


 class CustomBusinessMonthEnd(_CustomBusinessMonth):
-    __doc__ = _CustomBusinessMonth.__doc__.replace('[BEGIN/END]', 'end')
+    # TODO: Replace condition with Subsitution after dropping Py27 support


ideally we could standarize these todos so can find later, maybe TODO(py27), I think we are doing similar to EA t

WillAyd · 2018-05-30T00:01:09Z

AppVeyor failure is the random ResourceWarning that's been popping up which I was hoping would be fixed through #21105 but apparently was not. I don't believe its related to this change

jreback · 2018-05-30T10:31:24Z

ok this looks fine. @jorisvandenbossche do you have any comments.

jorisvandenbossche · 2018-05-30T21:04:58Z

Thanks!

(cherry picked from commit 4cbbcc6)

WillAyd added 2 commits May 16, 2018 14:22

Added test case

840eec8

Conditionals for -OO optimization support

19e71e3

jorisvandenbossche reviewed May 16, 2018

View reviewed changes

WillAyd added 2 commits May 16, 2018 15:11

Updated test name

95826d3

Py27 Compat

0e48c18

jreback requested changes May 16, 2018

View reviewed changes

jreback added the Compat pandas objects compatability with Numpy or Python functions label May 16, 2018

jreback requested changes May 16, 2018

View reviewed changes

jreback added this to the 0.23.1 milestone May 17, 2018

jreback added the Needs Backport label May 17, 2018

mattpap reviewed May 17, 2018

View reviewed changes

WillAyd added 3 commits May 20, 2018 12:14

Merge remote-tracking branch 'upstream/master' into oo-import

2248137

Leveraged docstring substitution

abdc5f2

Updated docstring for deprecate wrapper

e4ba948

WillAyd commented May 20, 2018

View reviewed changes

Whatsnew note

021786b

jreback reviewed May 23, 2018

View reviewed changes

WillAyd added 10 commits May 24, 2018 09:51

Reverted class Substitution in offsets.py

a1b409d

Recursive OO support

a89c4f4

LINT fixup

078610a

Merge remote-tracking branch 'upstream/master' into oo-import

91a2226

Updated whatsnew

c697dae

Reverted recursive import test

d6c66ec

Merge remote-tracking branch 'upstream/master' into oo-import

c853c67

Reverted conftest / decorators to master

c825636

Removed errant Categorical section in whatsnew

f8ccfa0

LINT fixup

e427f4b

jorisvandenbossche requested changes May 28, 2018

View reviewed changes

jreback approved these changes May 29, 2018

View reviewed changes

WillAyd added 3 commits May 29, 2018 13:28

Merge remote-tracking branch 'upstream/master' into oo-import

46a2f6a

Updated TODO comments

a2e748a

Fixed decorate docstring wrapper

75dbaad

jorisvandenbossche approved these changes May 30, 2018

View reviewed changes

jorisvandenbossche merged commit 4cbbcc6 into pandas-dev:master May 30, 2018

WillAyd deleted the oo-import branch May 30, 2018 22:03

jorisvandenbossche removed the Needs Backport label Jun 8, 2018

jorisvandenbossche pushed a commit to jorisvandenbossche/pandas that referenced this pull request Jun 8, 2018

BUG: Support for OO Optimization (pandas-dev#21093)

d861824

(cherry picked from commit 4cbbcc6)

jorisvandenbossche pushed a commit that referenced this pull request Jun 9, 2018

BUG: Support for OO Optimization (#21093)

ba16de0

(cherry picked from commit 4cbbcc6)

david-liu-brattle-1 pushed a commit to david-liu-brattle-1/pandas that referenced this pull request Jun 18, 2018

BUG: Support for OO Optimization (pandas-dev#21093)

b45fe1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for OO Optimization #21093

Support for OO Optimization #21093

WillAyd commented May 16, 2018 •

edited

Loading

jorisvandenbossche left a comment

jorisvandenbossche May 16, 2018

codecov bot commented May 16, 2018 •

edited

Loading

jreback May 16, 2018

WillAyd May 17, 2018

jreback left a comment

mattpap May 17, 2018

jorisvandenbossche May 23, 2018

WillAyd May 23, 2018

WillAyd May 25, 2018

jorisvandenbossche May 28, 2018

WillAyd May 20, 2018

jorisvandenbossche May 28, 2018

WillAyd commented May 22, 2018

jreback left a comment

jreback May 23, 2018

jorisvandenbossche May 28, 2018

jreback left a comment

jreback May 29, 2018

WillAyd commented May 30, 2018

jreback commented May 30, 2018

jorisvandenbossche commented May 30, 2018

		@@ -53,6 +55,15 @@ def test_xarray(df):
		assert df.to_xarray() is not None


		def test_xarray_oo_optimizable():

Support for OO Optimization #21093

Support for OO Optimization #21093

Conversation

WillAyd commented May 16, 2018 • edited Loading

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 16, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented May 22, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented May 30, 2018

jreback commented May 30, 2018

jorisvandenbossche commented May 30, 2018

WillAyd commented May 16, 2018 •

edited

Loading

codecov bot commented May 16, 2018 •

edited

Loading